Skip to content

BUG: Insert outline items in front of before and cover more of PdfWriter - #4415

Open
MartinThoma wants to merge 5 commits into
mainfrom
tst/writer-branch-coverage
Open

MartinThoma wants to merge 5 commits into
mainfrom
tst/writer-branch-coverage

Conversation

@MartinThoma

@MartinThoma MartinThoma commented Nov 12, 2026 •

Copy link
Copy Markdown
Member

PdfWriter.add_outline_item(before=item) only worked when item was the last item of its level.

TreeObject.insert_child() searched for before starting at /Last and following /Next, so it only ever found the last child. It now starts at /First when before is given, and updates /First when inserting in front of the first child. Appending without before still starts at /Last, so it stays O(1). A before that is not a child of the parent is still appended at the end, as before. #3965 also touches that part.

The PR also adds tests for PdfWriter branches without coverage so far:

  • add_outline_item() without a page (no /A action), and with a PageObject
  • the page_mode getter
  • add_annotation() with an invalid page type
  • clean_page() converting a named destination inside an /A action
  • merge() with excluded_fields=None, and with form fields that are on no merged page
  • reset_translation() for a reader the writer never copied from, and for an IndirectObject

Closes #4340.

@MartinThoma

Copy link
Copy Markdown
Member Author

I'll check if there is any comment from #4342 left to integrate

@codecov

codecov Bot commented Nov 12, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.05%. Comparing base (1e3e7e3) to head (4ce577b).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4415      +/-   ##
==========================================
+ Coverage   98.87%   99.05%   +0.17%     
==========================================
  Files          59       59              
  Lines       12299    12308       +9     
  Branches     2342     2345       +3     
==========================================
+ Hits        12161    12192      +31     
+ Misses         68       55      -13     
+ Partials       70       61       -9     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

…riter

PdfWriter.add_outline_item(before=item) appended the new item at the
end unless `item` was the last one of its level, and inserting in front
of the first item lost the new item: it was counted in /Count, but not
linked into the outline, as /First of the parent was not updated
(ISO 32000-2:2020, 12.3.3, Table 150). Closes #4340.

TreeObject.insert_child() looked for `before` starting at /Last and
following /Next, so it only found the last child. It now starts at
/First when `before` is given and updates /First when inserting in
front of the first child. Appending without `before` still starts at
/Last, so it stays O(1). A `before` that is no child of the parent
still appends at the end.

Add tests for PdfWriter branches without coverage so far:

* add_outline_item() with `before` being None, the first, a middle, the
  last or the only item, without a page (no /A action), and with a
  PageObject,
* the page_mode getter, add_annotation() with an invalid page type and
  clean_page() converting a named destination inside an /A action,
* merge() with excluded_fields=None and with form fields that are not
  on any merged page,
* reset_translation() for a reader the writer never copied from and for
  an IndirectObject, checked by appending the reader again and comparing
  the copied /Resources.

Where the expected behavior comes from ISO 32000-2:2020, the docstring
names the section.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@MartinThoma
MartinThoma force-pushed the tst/writer-branch-coverage branch from a19b6ef to 152ae7c Compare November 12, 2026 21:44
@MartinThoma MartinThoma changed the title TST: Cover more branches of PdfWriter BUG: Insert outline items in front of before and cover more of PdfWriter Nov 12, 2026
@MartinThoma
MartinThoma marked this pull request as ready for review November 13, 2026 14:06

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

add_outline_item(before=...) ignores before parameter

1 participant